NO-JIRA: test(e2e-v2): add ginkgo-based v2 test suite - #7152
Merged
Conversation
Add Ginkgo v2 testing framework and update related vendored dependencies including gomega matchers, protobuf, and supporting libraries needed for the new e2e v2 test suite. Signed-off-by: Cesar Wong <cewong@redhat.com> Assisted-by: Claude 3.7 Sonnet (via Claude Code)
Contributor
|
Skipping CI for Draft Pull Request. |
openshift-ci
Bot
added
do-not-merge/work-in-progress
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughAdds a v2 E2E testing framework: dependency updates (Ginkgo v2/Gomega), environment registry, test context, workload registry/resolver, CRD schema helper, version helpers, extensive API UX and control-plane workload tests, and YAML test assets. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
openshift-ci
Bot
added
the
area/testing
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: csrwng The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
approved
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
⛔ Files ignored due to path filters (269)
go.sumis excluded by!**/*.sumvendor/github.com/Masterminds/semver/v3/.gitignoreis excluded by!vendor/**,!**/vendor/**vendor/github.com/Masterminds/semver/v3/.golangci.ymlis excluded by!vendor/**,!**/vendor/**vendor/github.com/Masterminds/semver/v3/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/Masterminds/semver/v3/LICENSE.txtis excluded by!vendor/**,!**/vendor/**vendor/github.com/Masterminds/semver/v3/Makefileis excluded by!vendor/**,!**/vendor/**vendor/github.com/Masterminds/semver/v3/README.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/Masterminds/semver/v3/SECURITY.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/Masterminds/semver/v3/collection.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Masterminds/semver/v3/constraints.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Masterminds/semver/v3/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Masterminds/semver/v3/version.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-task/slim-sprig/v3/.editorconfigis excluded by!vendor/**,!**/vendor/**vendor/github.com/go-task/slim-sprig/v3/.gitattributesis excluded by!vendor/**,!**/vendor/**vendor/github.com/go-task/slim-sprig/v3/.gitignoreis excluded by!vendor/**,!**/vendor/**vendor/github.com/go-task/slim-sprig/v3/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/go-task/slim-sprig/v3/LICENSE.txtis excluded by!vendor/**,!**/vendor/**vendor/github.com/go-task/slim-sprig/v3/README.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/go-task/slim-sprig/v3/Taskfile.ymlis excluded by!vendor/**,!**/vendor/**vendor/github.com/go-task/slim-sprig/v3/crypto.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-task/slim-sprig/v3/date.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-task/slim-sprig/v3/defaults.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-task/slim-sprig/v3/dict.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-task/slim-sprig/v3/doc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-task/slim-sprig/v3/functions.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-task/slim-sprig/v3/list.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-task/slim-sprig/v3/network.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-task/slim-sprig/v3/numeric.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-task/slim-sprig/v3/reflect.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-task/slim-sprig/v3/regex.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-task/slim-sprig/v3/strings.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-task/slim-sprig/v3/url.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/.gitignoreis excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/CONTRIBUTING.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/Makefileis excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/README.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/RELEASING.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/config/deprecated.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/core_dsl.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/decorator_dsl.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/deprecated_dsl.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/formatter/colorable_others.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/formatter/colorable_windows.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/formatter/formatter.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/README.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/automaxprocs.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/cgroup.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/cgroups.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/cgroups2.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/cpu_quota_linux.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/cpu_quota_unsupported.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/errors.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/mountpoint.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/runtime.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/subsys.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/build/build_command.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/command/abort.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/command/command.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/command/program.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/generators/boostrap_templates.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/generators/bootstrap_command.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/generators/generate_command.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/generators/generate_templates.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/generators/generators_common.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/internal/compile.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/internal/gocovmerge.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/internal/profiles_and_reports.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/internal/run.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/internal/test_suite.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/internal/utils.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/internal/verify_version.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/labels/labels_command.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/main.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/outline/ginkgo.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/outline/import.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/outline/outline.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/outline/outline_command.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/run/run_command.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/unfocus/unfocus_command.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/watch/delta.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/watch/delta_tracker.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/watch/dependencies.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/watch/package_hash.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/watch/package_hashes.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/watch/suite.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo/watch/watch_command.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo_cli_dependencies.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/ginkgo_t_dsl.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/around_node.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/counter.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/failer.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/focus.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/global/init.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/group.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/interrupt_handler/interrupt_handler.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/interrupt_handler/sigquit_swallower_unix.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/interrupt_handler/sigquit_swallower_windows.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/node.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/ordering.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/output_interceptor.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/output_interceptor_unix.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/output_interceptor_wasm.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/output_interceptor_win.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/parallel_support/client_server.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/parallel_support/http_client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/parallel_support/http_server.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/parallel_support/rpc_client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/parallel_support/rpc_server.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/parallel_support/server_handler.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/progress_report.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/progress_report_bsd.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/progress_report_unix.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/progress_report_wasm.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/progress_report_win.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/progress_reporter_manager.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/report_entry.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/reporters/gojson.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/reporters/gojson_event_writer.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/reporters/gojson_reporter.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/spec.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/spec_context.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/suite.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/testingtproxy/testing_t_proxy.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/tree.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/internal/writer.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/reporters/default_reporter.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/reporters/deprecated_reporter.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/reporters/gojson_report.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/reporters/json_report.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/reporters/junit_report.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/reporters/reporter.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/reporters/teamcity_report.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/reporting_dsl.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/table_dsl.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/types/around_node.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/types/code_location.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/types/config.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/types/deprecated_types.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/types/deprecation_support.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/types/enum_support.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/types/errors.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/types/file_filter.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/types/flags.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/types/label_filter.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/types/report_entry.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/types/semver_filter.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/types/types.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/ginkgo/v2/types/version.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/gomega_dsl.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/internal/async_assertion.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/be_comparable_to_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/onsi/gomega/matchers/match_yaml_matcher.gois excluded by!vendor/**,!**/vendor/**vendor/go.yaml.in/yaml/v3/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/go.yaml.in/yaml/v3/NOTICEis excluded by!vendor/**,!**/vendor/**vendor/go.yaml.in/yaml/v3/README.mdis excluded by!vendor/**,!**/vendor/**vendor/go.yaml.in/yaml/v3/apic.gois excluded by!vendor/**,!**/vendor/**vendor/go.yaml.in/yaml/v3/decode.gois excluded by!vendor/**,!**/vendor/**vendor/go.yaml.in/yaml/v3/emitterc.gois excluded by!vendor/**,!**/vendor/**vendor/go.yaml.in/yaml/v3/encode.gois excluded by!vendor/**,!**/vendor/**vendor/go.yaml.in/yaml/v3/parserc.gois excluded by!vendor/**,!**/vendor/**vendor/go.yaml.in/yaml/v3/readerc.gois excluded by!vendor/**,!**/vendor/**vendor/go.yaml.in/yaml/v3/resolve.gois excluded by!vendor/**,!**/vendor/**vendor/go.yaml.in/yaml/v3/scannerc.gois excluded by!vendor/**,!**/vendor/**vendor/go.yaml.in/yaml/v3/sorter.gois excluded by!vendor/**,!**/vendor/**vendor/go.yaml.in/yaml/v3/writerc.gois excluded by!vendor/**,!**/vendor/**vendor/go.yaml.in/yaml/v3/yaml.gois excluded by!vendor/**,!**/vendor/**vendor/go.yaml.in/yaml/v3/yamlh.gois excluded by!vendor/**,!**/vendor/**vendor/go.yaml.in/yaml/v3/yamlprivateh.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/net/http2/http2.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/PATENTSis excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/cover/profile.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/ast/edge/edge.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/ast/inspector/cursor.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/ast/inspector/inspector.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/ast/inspector/iter.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/ast/inspector/typeof.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/ast/inspector/walk.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/gcexportdata/gcexportdata.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/gcexportdata/importer.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/packages/doc.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/packages/external.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/packages/golist.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/packages/golist_overlay.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/packages/loadmode_string.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/packages/packages.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/packages/visit.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/types/objectpath/objectpath.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/types/typeutil/callee.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/types/typeutil/imports.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/types/typeutil/map.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/types/typeutil/methodsetcache.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/go/types/typeutil/ui.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/aliases/aliases.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/aliases/aliases_go122.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/event/core/event.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/event/core/export.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/event/core/fast.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/event/doc.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/event/event.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/event/keys/keys.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/event/keys/standard.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/event/keys/util.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/event/label/label.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/gcimporter/bimport.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/gcimporter/exportdata.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/gcimporter/gcimporter.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/gcimporter/iexport.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/gcimporter/iimport.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/gcimporter/iimport_go122.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/gcimporter/predeclared.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/gcimporter/support.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/gcimporter/ureader_yes.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/gocommand/invoke.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/gocommand/invoke_notunix.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/gocommand/invoke_unix.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/gocommand/vendor.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/gocommand/version.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/packagesinternal/packages.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/pkgbits/codes.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/pkgbits/decoder.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/pkgbits/doc.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/pkgbits/encoder.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/pkgbits/flags.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/pkgbits/reloc.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/pkgbits/support.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/pkgbits/sync.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/pkgbits/syncmarker_string.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/pkgbits/version.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/stdlib/deps.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/stdlib/import.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/stdlib/manifest.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/stdlib/stdlib.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/typeparams/common.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/typeparams/coretype.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/typeparams/free.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/typeparams/normalize.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/typeparams/termlist.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/typeparams/typeterm.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/typesinternal/classify_call.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/typesinternal/element.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/typesinternal/errorcode.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/typesinternal/errorcode_string.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/typesinternal/qualifier.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/typesinternal/recv.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/typesinternal/toonew.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/typesinternal/types.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/typesinternal/varkind.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/typesinternal/zerovalue.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/versions/features.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/versions/gover.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/versions/types.gois excluded by!vendor/**,!**/vendor/**vendor/golang.org/x/tools/internal/versions/versions.gois excluded by!vendor/**,!**/vendor/**vendor/google.golang.org/protobuf/encoding/protowire/wire.gois excluded by!vendor/**,!**/vendor/**vendor/google.golang.org/protobuf/internal/editiondefaults/editions_defaults.binpbis excluded by!vendor/**,!**/vendor/**vendor/google.golang.org/protobuf/internal/filedesc/editions.gois excluded by!vendor/**,!**/vendor/**vendor/google.golang.org/protobuf/internal/filedesc/presence.gois excluded by!vendor/**,!**/vendor/**vendor/google.golang.org/protobuf/internal/genid/descriptor_gen.gois excluded by!vendor/**,!**/vendor/**vendor/google.golang.org/protobuf/internal/impl/codec_message_opaque.gois excluded by!vendor/**,!**/vendor/**vendor/google.golang.org/protobuf/internal/impl/message_opaque.gois excluded by!vendor/**,!**/vendor/**vendor/google.golang.org/protobuf/internal/impl/presence.gois excluded by!vendor/**,!**/vendor/**vendor/google.golang.org/protobuf/internal/version/version.gois excluded by!vendor/**,!**/vendor/**vendor/google.golang.org/protobuf/reflect/protoreflect/source_gen.gois excluded by!vendor/**,!**/vendor/**vendor/google.golang.org/protobuf/types/descriptorpb/descriptor.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**vendor/modules.txtis excluded by!vendor/**,!**/vendor/**
📒 Files selected for processing (14)
go.mod(5 hunks)test/e2e/util/crd.go(1 hunks)test/e2e/util/version.go(2 hunks)test/e2e/v2/internal/env_vars.go(1 hunks)test/e2e/v2/internal/test_context.go(1 hunks)test/e2e/v2/internal/workload_registry.go(1 hunks)test/e2e/v2/internal/workload_resolver.go(1 hunks)test/e2e/v2/tests/api_ux_validation_test.go(1 hunks)test/e2e/v2/tests/assets/hostedcluster-base.yaml(1 hunks)test/e2e/v2/tests/assets/karpenter-nodepool.yaml(1 hunks)test/e2e/v2/tests/assets/karpenter-workloads.yaml(1 hunks)test/e2e/v2/tests/assets/nodepool-base.yaml(1 hunks)test/e2e/v2/tests/control_plane_workloads_test.go(1 hunks)test/e2e/v2/tests/suite_test.go(1 hunks)
Comment on lines
+41
to
+80
| func hasFieldInSchema(schema *apiextensionsv1.JSONSchemaProps, pathParts []string, index int) bool { | ||
| if schema == nil || index >= len(pathParts) { | ||
| return index == len(pathParts) | ||
| } | ||
|
|
||
| currentPart := pathParts[index] | ||
|
|
||
| // Check properties first | ||
| if schema.Properties != nil { | ||
| if prop, exists := schema.Properties[currentPart]; exists { | ||
| if index == len(pathParts)-1 { | ||
| // This is the last part, field exists | ||
| return true | ||
| } | ||
| // Recurse into the property | ||
| return hasFieldInSchema(&prop, pathParts, index+1) | ||
| } | ||
| } | ||
|
|
||
| // Check AllOf, AnyOf, OneOf - these can contain the field | ||
| for i := range schema.AllOf { | ||
| if hasFieldInSchema(&schema.AllOf[i], pathParts, index) { | ||
| return true | ||
| } | ||
| } | ||
| for i := range schema.AnyOf { | ||
| if hasFieldInSchema(&schema.AnyOf[i], pathParts, index) { | ||
| return true | ||
| } | ||
| } | ||
| for i := range schema.OneOf { | ||
| if hasFieldInSchema(&schema.OneOf[i], pathParts, index) { | ||
| return true | ||
| } | ||
| } | ||
|
|
||
| // Check if there's a $ref that we need to follow | ||
| // Note: In CRDs, $ref typically points to definitions within the same schema | ||
| // For simplicity, we check properties which is the most common case | ||
| return false |
Contributor
There was a problem hiding this comment.
Handle array members when traversing schemas. Today any path that dives into an array (for example status.conditions.type) is reported missing because we never follow schema.Items. That causes false negatives in the new helpers and will break tests that rely on legitimate CRD fields living inside arrays. Please descend into Items without advancing the path index, e.g.:
@@
- // Check AllOf, AnyOf, OneOf - these can contain the field
+ // Bridge into array items before falling back to the combinators.
+ if schema.Items != nil {
+ if schema.Items.Schema != nil && hasFieldInSchema(schema.Items.Schema, pathParts, index) {
+ return true
+ }
+ for i := range schema.Items.JSONSchemas {
+ if hasFieldInSchema(&schema.Items.JSONSchemas[i], pathParts, index) {
+ return true
+ }
+ }
+ }
+
+ // Check AllOf, AnyOf, OneOf - these can contain the field📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| func hasFieldInSchema(schema *apiextensionsv1.JSONSchemaProps, pathParts []string, index int) bool { | |
| if schema == nil || index >= len(pathParts) { | |
| return index == len(pathParts) | |
| } | |
| currentPart := pathParts[index] | |
| // Check properties first | |
| if schema.Properties != nil { | |
| if prop, exists := schema.Properties[currentPart]; exists { | |
| if index == len(pathParts)-1 { | |
| // This is the last part, field exists | |
| return true | |
| } | |
| // Recurse into the property | |
| return hasFieldInSchema(&prop, pathParts, index+1) | |
| } | |
| } | |
| // Check AllOf, AnyOf, OneOf - these can contain the field | |
| for i := range schema.AllOf { | |
| if hasFieldInSchema(&schema.AllOf[i], pathParts, index) { | |
| return true | |
| } | |
| } | |
| for i := range schema.AnyOf { | |
| if hasFieldInSchema(&schema.AnyOf[i], pathParts, index) { | |
| return true | |
| } | |
| } | |
| for i := range schema.OneOf { | |
| if hasFieldInSchema(&schema.OneOf[i], pathParts, index) { | |
| return true | |
| } | |
| } | |
| // Check if there's a $ref that we need to follow | |
| // Note: In CRDs, $ref typically points to definitions within the same schema | |
| // For simplicity, we check properties which is the most common case | |
| return false | |
| func hasFieldInSchema(schema *apiextensionsv1.JSONSchemaProps, pathParts []string, index int) bool { | |
| if schema == nil || index >= len(pathParts) { | |
| return index == len(pathParts) | |
| } | |
| currentPart := pathParts[index] | |
| // Check properties first | |
| if schema.Properties != nil { | |
| if prop, exists := schema.Properties[currentPart]; exists { | |
| if index == len(pathParts)-1 { | |
| // This is the last part, field exists | |
| return true | |
| } | |
| // Recurse into the property | |
| return hasFieldInSchema(&prop, pathParts, index+1) | |
| } | |
| } | |
| // Bridge into array items before falling back to the combinators. | |
| if schema.Items != nil { | |
| if schema.Items.Schema != nil && hasFieldInSchema(schema.Items.Schema, pathParts, index) { | |
| return true | |
| } | |
| for i := range schema.Items.JSONSchemas { | |
| if hasFieldInSchema(&schema.Items.JSONSchemas[i], pathParts, index) { | |
| return true | |
| } | |
| } | |
| } | |
| // Check AllOf, AnyOf, OneOf - these can contain the field | |
| for i := range schema.AllOf { | |
| if hasFieldInSchema(&schema.AllOf[i], pathParts, index) { | |
| return true | |
| } | |
| } | |
| for i := range schema.AnyOf { | |
| if hasFieldInSchema(&schema.AnyOf[i], pathParts, index) { | |
| return true | |
| } | |
| } | |
| for i := range schema.OneOf { | |
| if hasFieldInSchema(&schema.OneOf[i], pathParts, index) { | |
| return true | |
| } | |
| } | |
| // Check if there's a $ref that we need to follow | |
| // Note: In CRDs, $ref typically points to definitions within the same schema | |
| // For simplicity, we check properties which is the most common case | |
| return false |
🤖 Prompt for AI Agents
In test/e2e/util/crd.go around lines 41 to 80, the schema traversal currently
ignores array members so any path that descends into an array (e.g.
status.conditions.type) is reported missing; update the function to descend into
schema.Items without advancing the path index: if schema.Items != nil then if
schema.Items.Schema != nil call hasFieldInSchema(schema.Items.Schema, pathParts,
index) and return true if it matches; also handle the case where
schema.Items.JSONSchemas (array form) exists by iterating those entries and
calling hasFieldInSchema on each with the same index; return true if any match.
Ensure you do not increment index when recursing into Items.
Comment on lines
+64
to
+77
| func SetReleaseVersionFromHostedCluster(ctx context.Context, hostedCluster *hyperv1.HostedCluster) error { | ||
| if len(hostedCluster.Status.Version.History) == 0 { | ||
| return fmt.Errorf("hostedCluster has no version history") | ||
| } | ||
| hcVersion := hostedCluster.Status.Version.History[0].Version | ||
| var err error | ||
| releaseVersion, err = semver.Parse(hcVersion) | ||
| if err != nil { | ||
| return fmt.Errorf("error parsing version: %v", err) | ||
| } | ||
| releaseVersion.Patch = 0 | ||
| releaseVersion.Pre = nil | ||
| releaseVersion.Build = nil | ||
| return nil |
Contributor
There was a problem hiding this comment.
Guard against nil HostedCluster status version
SetReleaseVersionFromHostedCluster dereferences hostedCluster.Status.Version.History without checking whether Status.Version is nil. When the status hasn't been populated yet (common early in cluster lifecycle), this will panic before we can return an error. Please add an explicit nil check (and bail early with an error) before touching the history slice.
func SetReleaseVersionFromHostedCluster(ctx context.Context, hostedCluster *hyperv1.HostedCluster) error {
- if len(hostedCluster.Status.Version.History) == 0 {
+ if hostedCluster.Status.Version == nil {
+ return fmt.Errorf("hostedCluster has no version status")
+ }
+ if len(hostedCluster.Status.Version.History) == 0 {
return fmt.Errorf("hostedCluster has no version history")
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| func SetReleaseVersionFromHostedCluster(ctx context.Context, hostedCluster *hyperv1.HostedCluster) error { | |
| if len(hostedCluster.Status.Version.History) == 0 { | |
| return fmt.Errorf("hostedCluster has no version history") | |
| } | |
| hcVersion := hostedCluster.Status.Version.History[0].Version | |
| var err error | |
| releaseVersion, err = semver.Parse(hcVersion) | |
| if err != nil { | |
| return fmt.Errorf("error parsing version: %v", err) | |
| } | |
| releaseVersion.Patch = 0 | |
| releaseVersion.Pre = nil | |
| releaseVersion.Build = nil | |
| return nil | |
| func SetReleaseVersionFromHostedCluster(ctx context.Context, hostedCluster *hyperv1.HostedCluster) error { | |
| if hostedCluster.Status.Version == nil { | |
| return fmt.Errorf("hostedCluster has no version status") | |
| } | |
| if len(hostedCluster.Status.Version.History) == 0 { | |
| return fmt.Errorf("hostedCluster has no version history") | |
| } | |
| hcVersion := hostedCluster.Status.Version.History[0].Version | |
| var err error | |
| releaseVersion, err = semver.Parse(hcVersion) | |
| if err != nil { | |
| return fmt.Errorf("error parsing version: %v", err) | |
| } | |
| releaseVersion.Patch = 0 | |
| releaseVersion.Pre = nil | |
| releaseVersion.Build = nil | |
| return nil |
🤖 Prompt for AI Agents
In test/e2e/util/version.go around lines 64 to 77, the function dereferences
hostedCluster.Status.Version.History without checking for nil; add explicit nil
checks for hostedCluster.Status and hostedCluster.Status.Version (and optionally
hostedCluster.Status.Version.History) before accessing History and return a
descriptive error if any are nil to avoid a panic. Update the beginning of the
function to validate hostedCluster.Status != nil and
hostedCluster.Status.Version != nil (and that History exists/has length) and
bail out with fmt.Errorf(...) if the checks fail, leaving the rest of the
parsing logic unchanged.
Comment on lines
+168
to
+199
| if owner.Kind == workload.Kind && owner.Name == workload.Name && ownerUID == workload.UID { | ||
| return true | ||
| } | ||
|
|
||
| // If we found a top-level workload type (Deployment, StatefulSet, CronJob, standalone Job), stop | ||
| if owner.Kind == "Deployment" || owner.Kind == "StatefulSet" || owner.Kind == "CronJob" || owner.Kind == "Job" { | ||
| break | ||
| } | ||
|
|
||
| // Follow the chain for ReplicaSet -> Deployment or Job -> CronJob | ||
| var nextOwner *metav1.OwnerReference | ||
| switch owner.Kind { | ||
| case "ReplicaSet": | ||
| rs := &appsv1.ReplicaSet{} | ||
| if err := client.Get(ctx, types.NamespacedName{Namespace: pod.Namespace, Name: owner.Name}, rs); err != nil { | ||
| break | ||
| } | ||
| if len(rs.OwnerReferences) > 0 && rs.OwnerReferences[0].Kind == "Deployment" { | ||
| nextOwner = &rs.OwnerReferences[0] | ||
| } | ||
| case "Job": | ||
| job := &batchv1.Job{} | ||
| if err := client.Get(ctx, types.NamespacedName{Namespace: pod.Namespace, Name: owner.Name}, job); err != nil { | ||
| break | ||
| } | ||
| if len(job.OwnerReferences) > 0 && job.OwnerReferences[0].Kind == "CronJob" { | ||
| nextOwner = &job.OwnerReferences[0] | ||
| } | ||
| default: | ||
| break | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
Allow traversal from Jobs up to CronJobs.
belongsToWorkload breaks out as soon as it sees an owner of kind Job, so CronJob-backed pods never match their workload and all CronJob tests silently skip. Remove Job from the early break so the subsequent traversal can reach and verify the CronJob owner.
- // If we found a top-level workload type (Deployment, StatefulSet, CronJob, standalone Job), stop
- if owner.Kind == "Deployment" || owner.Kind == "StatefulSet" || owner.Kind == "CronJob" || owner.Kind == "Job" {
+ // If we found a top-level workload type (Deployment, StatefulSet, CronJob), stop
+ if owner.Kind == "Deployment" || owner.Kind == "StatefulSet" || owner.Kind == "CronJob" {
break
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if owner.Kind == workload.Kind && owner.Name == workload.Name && ownerUID == workload.UID { | |
| return true | |
| } | |
| // If we found a top-level workload type (Deployment, StatefulSet, CronJob, standalone Job), stop | |
| if owner.Kind == "Deployment" || owner.Kind == "StatefulSet" || owner.Kind == "CronJob" || owner.Kind == "Job" { | |
| break | |
| } | |
| // Follow the chain for ReplicaSet -> Deployment or Job -> CronJob | |
| var nextOwner *metav1.OwnerReference | |
| switch owner.Kind { | |
| case "ReplicaSet": | |
| rs := &appsv1.ReplicaSet{} | |
| if err := client.Get(ctx, types.NamespacedName{Namespace: pod.Namespace, Name: owner.Name}, rs); err != nil { | |
| break | |
| } | |
| if len(rs.OwnerReferences) > 0 && rs.OwnerReferences[0].Kind == "Deployment" { | |
| nextOwner = &rs.OwnerReferences[0] | |
| } | |
| case "Job": | |
| job := &batchv1.Job{} | |
| if err := client.Get(ctx, types.NamespacedName{Namespace: pod.Namespace, Name: owner.Name}, job); err != nil { | |
| break | |
| } | |
| if len(job.OwnerReferences) > 0 && job.OwnerReferences[0].Kind == "CronJob" { | |
| nextOwner = &job.OwnerReferences[0] | |
| } | |
| default: | |
| break | |
| } | |
| if owner.Kind == workload.Kind && owner.Name == workload.Name && ownerUID == workload.UID { | |
| return true | |
| } | |
| // If we found a top-level workload type (Deployment, StatefulSet, CronJob), stop | |
| if owner.Kind == "Deployment" || owner.Kind == "StatefulSet" || owner.Kind == "CronJob" { | |
| break | |
| } | |
| // Follow the chain for ReplicaSet -> Deployment or Job -> CronJob | |
| var nextOwner *metav1.OwnerReference | |
| switch owner.Kind { | |
| case "ReplicaSet": | |
| rs := &appsv1.ReplicaSet{} | |
| if err := client.Get(ctx, types.NamespacedName{Namespace: pod.Namespace, Name: owner.Name}, rs); err != nil { | |
| break | |
| } | |
| if len(rs.OwnerReferences) > 0 && rs.OwnerReferences[0].Kind == "Deployment" { | |
| nextOwner = &rs.OwnerReferences[0] | |
| } | |
| case "Job": | |
| job := &batchv1.Job{} | |
| if err := client.Get(ctx, types.NamespacedName{Namespace: pod.Namespace, Name: owner.Name}, job); err != nil { | |
| break | |
| } | |
| if len(job.OwnerReferences) > 0 && job.OwnerReferences[0].Kind == "CronJob" { | |
| nextOwner = &job.OwnerReferences[0] | |
| } | |
| default: | |
| break | |
| } |
🤖 Prompt for AI Agents
In test/e2e/v2/internal/workload_resolver.go around lines 168 to 199, the early
exit checks include "Job" which prevents traversal from Job -> CronJob and
causes CronJob-backed pods to be missed; remove "Job" from the top-level break
condition (leave Deployment, StatefulSet, CronJob only) so the switch that
follows can resolve Job owners up to CronJob, keeping the existing ReplicaSet
and Job lookup logic intact.
Comment on lines
+1311
to
+1317
| It("should reject when image is empty", func() { | ||
| err := testNodePoolCreation(ctx, mgmtClient, "nodepool-base.yaml", func(np *hyperv1.NodePool) { | ||
| np.Spec.Release.Image = "@" | ||
| }) | ||
| Expect(err).To(HaveOccurred()) | ||
| Expect(err.Error()).To(ContainSubstring("Image must start with a word character (letters, digits, or underscores) and contain no white spaces")) | ||
| }) |
Contributor
There was a problem hiding this comment.
Fix “image is empty” mutation
This case is meant to cover an empty release image, but it still sets the image to "@", duplicating the preceding “bad format” test. As written, we never assert the empty-string path, so a regression there would slip through.
- np.Spec.Release.Image = "@"
+ np.Spec.Release.Image = ""📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| It("should reject when image is empty", func() { | |
| err := testNodePoolCreation(ctx, mgmtClient, "nodepool-base.yaml", func(np *hyperv1.NodePool) { | |
| np.Spec.Release.Image = "@" | |
| }) | |
| Expect(err).To(HaveOccurred()) | |
| Expect(err.Error()).To(ContainSubstring("Image must start with a word character (letters, digits, or underscores) and contain no white spaces")) | |
| }) | |
| It("should reject when image is empty", func() { | |
| err := testNodePoolCreation(ctx, mgmtClient, "nodepool-base.yaml", func(np *hyperv1.NodePool) { | |
| np.Spec.Release.Image = "" | |
| }) | |
| Expect(err).To(HaveOccurred()) | |
| Expect(err.Error()).To(ContainSubstring("Image must start with a word character (letters, digits, or underscores) and contain no white spaces")) | |
| }) |
🤖 Prompt for AI Agents
In test/e2e/v2/tests/api_ux_validation_test.go around lines 1311 to 1317, the
test intended to validate an empty release image mistakenly sets
np.Spec.Release.Image = "@" (duplicating the bad-format case); change the
mutation to set np.Spec.Release.Image = "" to exercise the empty-string path,
and update the Expect assertion to check for the empty-image error message
(e.g., assert the error mentions that the image must not be empty or similar
project-specific empty-field text).
Comment on lines
+29
to
+49
| services: | ||
| - service: APIServer | ||
| servicePublishingStrategy: | ||
| type: Route | ||
| route: | ||
| hostname: api.agarcial.hypershift.devcluster.openshift.com | ||
| - service: OAuthServer | ||
| servicePublishingStrategy: | ||
| type: Route | ||
| route: | ||
| hostname: OAuthServer.agarcial.hypershift.devcluster.openshift.com | ||
| - service: Konnectivity | ||
| servicePublishingStrategy: | ||
| type: Route | ||
| route: | ||
| hostname: Konnectivity.agarcial.hypershift.devcluster.openshift.com | ||
| - service: Ignition | ||
| servicePublishingStrategy: | ||
| type: Route | ||
| route: | ||
|
|
Contributor
There was a problem hiding this comment.
Fix invalid route hostnames. The OAuthServer and Konnectivity routes currently use capital letters, and Ignition leaves hostname empty. DNS-1123 hostnames must be lowercase and non-empty, otherwise the API will reject the manifest before the suite ever runs. You’ll want to provide fully lowercased values (for example oauthserver.agarcial…, konnectivity.agarcial…) and either supply a valid hostname for Ignition or drop the field entirely so Hypershift can infer one. (kubernetes.io)
🤖 Prompt for AI Agents
In test/e2e/v2/tests/assets/hostedcluster-base.yaml around lines 29 to 49, the
Route hostnames for OAuthServer and Konnectivity use capital letters and
Ignition's hostname is empty, violating DNS-1123 rules; update the OAuthServer
and Konnectivity route hostnames to lowercase (e.g.
oauthserver.agarcial.hypershift.devcluster.openshift.com and
konnectivity.agarcial.hypershift.devcluster.openshift.com) and either supply a
valid non-empty, lowercased hostname for Ignition (e.g.
ignition.agarcial.hypershift.devcluster.openshift.com) or remove the empty
hostname field so Hypershift can infer it.
Comment on lines
+147
to
+195
| for _, workload := range workloads { | ||
| It(fmt.Sprintf("should exist for pods with emptyDir or hostPath volumes belonging to %s", workload.Name), func() { | ||
| if shouldSkipWorkload(workload) { | ||
| Skip(fmt.Sprintf("workload %s is platform-specific and doesn't match cluster platform", workload.Name)) | ||
| } | ||
|
|
||
| // Skip if workload is in exemption list | ||
| if slices.Contains(exemptions, workload.Name) { | ||
| Skip(fmt.Sprintf("workload %s is exempt from safe-to-evict annotations check", workload.Name)) | ||
| } | ||
|
|
||
| pods := getWorkloadPods(workload) | ||
| if len(pods) == 0 { | ||
| Skip(fmt.Sprintf("no pods found for workload %s", workload.Name)) | ||
| } | ||
|
|
||
| for _, pod := range pods { | ||
| // Check if pod has emptyDir or hostPath volumes | ||
| hasLocalVolumes := false | ||
| var localVolumeNames []string | ||
| for _, volume := range pod.Spec.Volumes { | ||
| if volume.EmptyDir != nil || volume.HostPath != nil { | ||
| hasLocalVolumes = true | ||
| localVolumeNames = append(localVolumeNames, volume.Name) | ||
| } | ||
| } | ||
|
|
||
| if hasLocalVolumes { | ||
| annotationKey := "cluster-autoscaler.kubernetes.io/safe-to-evict-local-volumes" | ||
| annotationValue, exists := pod.Annotations[annotationKey] | ||
| Expect(exists).To(BeTrue(), "pod %s has local volumes but missing safe-to-evict annotation", pod.Name) | ||
| Expect(annotationValue).NotTo(BeEmpty(), "pod %s has empty safe-to-evict annotation", pod.Name) | ||
|
|
||
| // Verify all local volumes are listed in annotation | ||
| annotatedVolumes := strings.Split(annotationValue, ",") | ||
| for _, volName := range localVolumeNames { | ||
| found := false | ||
| for _, annVol := range annotatedVolumes { | ||
| if strings.TrimSpace(annVol) == volName { | ||
| found = true | ||
| break | ||
| } | ||
| } | ||
| Expect(found).To(BeTrue(), "pod %s local volume %s not found in annotation", pod.Name, volName) | ||
| } | ||
| } | ||
| } | ||
| }) | ||
| } |
Contributor
There was a problem hiding this comment.
Restore per-test workload capture in safe-to-evict checks.
This It closure reuses the outer loop variable, so every test ends up exercising only the final workload when the module isn’t built with Go ≥1.22 semantics. Rebind the variable inside the loop to keep the cases independent.
- for _, workload := range workloads {
+ for _, workload := range workloads {
+ workload := workload // capture loop variable
It(fmt.Sprintf("should exist for pods with emptyDir or hostPath volumes belonging to %s", workload.Name), func() {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| for _, workload := range workloads { | |
| It(fmt.Sprintf("should exist for pods with emptyDir or hostPath volumes belonging to %s", workload.Name), func() { | |
| if shouldSkipWorkload(workload) { | |
| Skip(fmt.Sprintf("workload %s is platform-specific and doesn't match cluster platform", workload.Name)) | |
| } | |
| // Skip if workload is in exemption list | |
| if slices.Contains(exemptions, workload.Name) { | |
| Skip(fmt.Sprintf("workload %s is exempt from safe-to-evict annotations check", workload.Name)) | |
| } | |
| pods := getWorkloadPods(workload) | |
| if len(pods) == 0 { | |
| Skip(fmt.Sprintf("no pods found for workload %s", workload.Name)) | |
| } | |
| for _, pod := range pods { | |
| // Check if pod has emptyDir or hostPath volumes | |
| hasLocalVolumes := false | |
| var localVolumeNames []string | |
| for _, volume := range pod.Spec.Volumes { | |
| if volume.EmptyDir != nil || volume.HostPath != nil { | |
| hasLocalVolumes = true | |
| localVolumeNames = append(localVolumeNames, volume.Name) | |
| } | |
| } | |
| if hasLocalVolumes { | |
| annotationKey := "cluster-autoscaler.kubernetes.io/safe-to-evict-local-volumes" | |
| annotationValue, exists := pod.Annotations[annotationKey] | |
| Expect(exists).To(BeTrue(), "pod %s has local volumes but missing safe-to-evict annotation", pod.Name) | |
| Expect(annotationValue).NotTo(BeEmpty(), "pod %s has empty safe-to-evict annotation", pod.Name) | |
| // Verify all local volumes are listed in annotation | |
| annotatedVolumes := strings.Split(annotationValue, ",") | |
| for _, volName := range localVolumeNames { | |
| found := false | |
| for _, annVol := range annotatedVolumes { | |
| if strings.TrimSpace(annVol) == volName { | |
| found = true | |
| break | |
| } | |
| } | |
| Expect(found).To(BeTrue(), "pod %s local volume %s not found in annotation", pod.Name, volName) | |
| } | |
| } | |
| } | |
| }) | |
| } | |
| for _, workload := range workloads { | |
| workload := workload // capture loop variable | |
| It(fmt.Sprintf("should exist for pods with emptyDir or hostPath volumes belonging to %s", workload.Name), func() { | |
| if shouldSkipWorkload(workload) { | |
| Skip(fmt.Sprintf("workload %s is platform-specific and doesn't match cluster platform", workload.Name)) | |
| } | |
| // Skip if workload is in exemption list | |
| if slices.Contains(exemptions, workload.Name) { | |
| Skip(fmt.Sprintf("workload %s is exempt from safe-to-evict annotations check", workload.Name)) | |
| } | |
| pods := getWorkloadPods(workload) | |
| if len(pods) == 0 { | |
| Skip(fmt.Sprintf("no pods found for workload %s", workload.Name)) | |
| } | |
| for _, pod := range pods { | |
| // Check if pod has emptyDir or hostPath volumes | |
| hasLocalVolumes := false | |
| var localVolumeNames []string | |
| for _, volume := range pod.Spec.Volumes { | |
| if volume.EmptyDir != nil || volume.HostPath != nil { | |
| hasLocalVolumes = true | |
| localVolumeNames = append(localVolumeNames, volume.Name) | |
| } | |
| } | |
| if hasLocalVolumes { | |
| annotationKey := "cluster-autoscaler.kubernetes.io/safe-to-evict-local-volumes" | |
| annotationValue, exists := pod.Annotations[annotationKey] | |
| Expect(exists).To(BeTrue(), "pod %s has local volumes but missing safe-to-evict annotation", pod.Name) | |
| Expect(annotationValue).NotTo(BeEmpty(), "pod %s has empty safe-to-evict annotation", pod.Name) | |
| // Verify all local volumes are listed in annotation | |
| annotatedVolumes := strings.Split(annotationValue, ",") | |
| for _, volName := range localVolumeNames { | |
| found := false | |
| for _, annVol := range annotatedVolumes { | |
| if strings.TrimSpace(annVol) == volName { | |
| found = true | |
| break | |
| } | |
| } | |
| Expect(found).To(BeTrue(), "pod %s local volume %s not found in annotation", pod.Name, volName) | |
| } | |
| } | |
| } | |
| }) | |
| } |
🤖 Prompt for AI Agents
In test/e2e/v2/tests/control_plane_workloads_test.go around lines 147 to 195,
the It closure captures the loop variable workload causing all tests to
reference the final workload; rebind the variable before creating the closure
(e.g., w := workload) and use w in the It description and inside the test body
(getWorkloadPods(w), w.Name, etc.) so each test captures its own workload value.
Comment on lines
+664
to
+683
|
|
||
| hasColocationAffinity := false | ||
| for _, term := range pod.Spec.Affinity.PodAffinity.PreferredDuringSchedulingIgnoredDuringExecution { | ||
| if term.Weight == 100 { | ||
| for _, req := range term.PodAffinityTerm.LabelSelector.MatchLabels { | ||
| if req == testCtx.ControlPlaneNamespace { | ||
| hasColocationAffinity = true | ||
| break | ||
| } | ||
| } | ||
| if term.PodAffinityTerm.LabelSelector != nil { | ||
| if term.PodAffinityTerm.LabelSelector.MatchLabels[colocationLabelKey] == testCtx.ControlPlaneNamespace { | ||
| hasColocationAffinity = true | ||
| break | ||
| } | ||
| } | ||
| } | ||
| } | ||
| Expect(hasColocationAffinity).To(BeTrue(), "pod %s should have colocation pod affinity", pod.Name) | ||
| } |
Contributor
There was a problem hiding this comment.
Guard PodAffinity label selectors before dereferencing.
Several control-plane pods omit PodAffinityTerm.LabelSelector; this loop dereferences it unconditionally and will panic the test binary. Gate the checks behind a nil guard so we fail via Expect instead of crashing.
- hasColocationAffinity := false
- for _, term := range pod.Spec.Affinity.PodAffinity.PreferredDuringSchedulingIgnoredDuringExecution {
- if term.Weight == 100 {
- for _, req := range term.PodAffinityTerm.LabelSelector.MatchLabels {
- if req == testCtx.ControlPlaneNamespace {
- hasColocationAffinity = true
- break
- }
- }
- if term.PodAffinityTerm.LabelSelector != nil {
- if term.PodAffinityTerm.LabelSelector.MatchLabels[colocationLabelKey] == testCtx.ControlPlaneNamespace {
- hasColocationAffinity = true
- break
- }
- }
- }
- }
+ hasColocationAffinity := false
+ for _, term := range pod.Spec.Affinity.PodAffinity.PreferredDuringSchedulingIgnoredDuringExecution {
+ if term.Weight != 100 || term.PodAffinityTerm.LabelSelector == nil {
+ continue
+ }
+ for _, value := range term.PodAffinityTerm.LabelSelector.MatchLabels {
+ if value == testCtx.ControlPlaneNamespace {
+ hasColocationAffinity = true
+ break
+ }
+ }
+ if term.PodAffinityTerm.LabelSelector.MatchLabels[colocationLabelKey] == testCtx.ControlPlaneNamespace {
+ hasColocationAffinity = true
+ break
+ }
+ }🤖 Prompt for AI Agents
In test/e2e/v2/tests/control_plane_workloads_test.go around lines 664 to 683,
the loop dereferences term.PodAffinityTerm.LabelSelector without checking for
nil which can panic when LabelSelector is absent; update the loop to first check
if term.PodAffinityTerm != nil and term.PodAffinityTerm.LabelSelector != nil
before accessing MatchLabels, and only then inspect MatchLabels and the
colocationLabelKey; leave the Expect(hasColocationAffinity) assertion as-is so
pods missing the selector will cause a test failure rather than a runtime panic.
csrwng
marked this pull request as ready for review
November 7, 2025 01:45
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (6)
test/e2e/util/crd.go (1)
48-75: Cover array items during schema traversal.
Line 60 still skipsschema.Items, so any path that passes through a list (e.g.status.conditions.type) is always reported missing. That breaks consumers ofHasFieldInCRDSchema. Please descend intoItems.SchemaandItems.JSONSchemaswithout advancing the path index before falling back to the combinators. Example fix:@@ - // Check AllOf, AnyOf, OneOf - these can contain the field + // Bridge into array items before other combinators + if schema.Items != nil { + if schema.Items.Schema != nil && hasFieldInSchema(schema.Items.Schema, pathParts, index) { + return true + } + for i := range schema.Items.JSONSchemas { + if hasFieldInSchema(&schema.Items.JSONSchemas[i], pathParts, index) { + return true + } + } + } + + // Check AllOf, AnyOf, OneOf - these can contain the fieldtest/e2e/v2/tests/assets/hostedcluster-base.yaml (1)
35-49: Fix invalid Route hostnames.
Lines 39-49 still violate DNS-1123: the OAuthServer/Konnectivity hostnames contain uppercase letters and Ignition leaveshostnameempty, so the manifest is rejected before tests run. Lowercase the hostnames and supply (or omit) Ignition’s hostname. For example:@@ - hostname: OAuthServer.agarcial.hypershift.devcluster.openshift.com + hostname: oauthserver.agarcial.hypershift.devcluster.openshift.com @@ - hostname: Konnectivity.agarcial.hypershift.devcluster.openshift.com + hostname: konnectivity.agarcial.hypershift.devcluster.openshift.com @@ - route: - + route: + hostname: ignition.agarcial.hypershift.devcluster.openshift.comtest/e2e/util/version.go (1)
64-78: Guard against nil HostedCluster version status.
Line 65 still assumeshostedCluster.Status.Versionexists. When status hasn’t populated yet, this dereference panics before you can log the warning. Add nil checks before touchingHistory, e.g.:func SetReleaseVersionFromHostedCluster(ctx context.Context, hostedCluster *hyperv1.HostedCluster) error { - if len(hostedCluster.Status.Version.History) == 0 || hostedCluster.Status.Version.History[0].Version == "" { + if hostedCluster.Status.Version == nil { + ginkgo.GinkgoWriter.Write([]byte("WARNING: hostedCluster status has no version information")) + return nil + } + if len(hostedCluster.Status.Version.History) == 0 || hostedCluster.Status.Version.History[0].Version == "" { ginkgo.GinkgoWriter.Write([]byte("WARNING: cannot determine release version from HostedCluster")) return nil }test/e2e/v2/tests/api_ux_validation_test.go (1)
1303-1316: Fix the empty-image mutationThis “image is empty” case still sets the release image to
"@", so it duplicates the bad-format test and never exercises the empty-string validation path. Set the mutation to""(and update the expectation to assert the empty-image error) so regressions in that branch are caught.- It("should reject when image is empty", func() { + It("should reject when image is empty", func() { err := testNodePoolCreation(ctx, mgmtClient, "nodepool-base.yaml", func(np *hyperv1.NodePool) { - np.Spec.Release.Image = "@" + np.Spec.Release.Image = "" }) Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("Image must start with a word character (letters, digits, or underscores) and contain no white spaces")) + Expect(err.Error()).To(ContainSubstring("Image must not be empty")) })test/e2e/v2/tests/control_plane_workloads_test.go (2)
79-186: Capture the workload per test registrationEach
Itclosure still closes over the loop’sworkloadvariable. On Go releases prior to 1.22—or modules that haven’t opted into the new semantics—all registered specs run against the final workload only. Rebind inside the loop (workload := workload) before declaring theItso every test captures its own case. Apply the same fix across the other workload loops.- for _, workload := range workloads { + for _, workload := range workloads { + workload := workload if workload.Type != "Deployment" { continue }
680-698: Guard nil PodAffinity label selectorsSome control-plane pods omit
PodAffinityTerm.LabelSelector; this loop dereferences it before checking, which panics the test binary. Bail out early whenPodAffinityTermor itsLabelSelectoris nil, and only then inspectMatchLabels.- for _, term := range pod.Spec.Affinity.PodAffinity.PreferredDuringSchedulingIgnoredDuringExecution { - if term.Weight == 100 { - for _, req := range term.PodAffinityTerm.LabelSelector.MatchLabels { + for _, term := range pod.Spec.Affinity.PodAffinity.PreferredDuringSchedulingIgnoredDuringExecution { + if term.Weight != 100 || term.PodAffinityTerm == nil || term.PodAffinityTerm.LabelSelector == nil { + continue + } + for _, value := range term.PodAffinityTerm.LabelSelector.MatchLabels { - if req == testCtx.ControlPlaneNamespace { + if value == testCtx.ControlPlaneNamespace { hasColocationAffinity = true break } } - if term.PodAffinityTerm.LabelSelector != nil { - if term.PodAffinityTerm.LabelSelector.MatchLabels[colocationLabelKey] == testCtx.ControlPlaneNamespace { - hasColocationAffinity = true - break - } - } + if term.PodAffinityTerm.LabelSelector.MatchLabels[colocationLabelKey] == testCtx.ControlPlaneNamespace { + hasColocationAffinity = true + break + } } }
🧹 Nitpick comments (1)
test/e2e/v2/tests/assets/nodepool-base.yaml (1)
4-5: Clean up empty annotation and label fields.Lines 4-5 define
annotations:andlabels:with no values, which creates implicit null entries. For clarity and to match Kubernetes conventions, either remove these lines or use explicit empty object syntax (annotations: {}).Apply this diff:
metadata: - annotations: - labels: name: baseAlternatively, if you prefer to keep the structure as a reminder for future expansion:
metadata: - annotations: - labels: + annotations: {} + labels: {} name: base
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (13)
test/e2e/util/crd.go(1 hunks)test/e2e/util/version.go(2 hunks)test/e2e/v2/internal/env_vars.go(1 hunks)test/e2e/v2/internal/test_context.go(1 hunks)test/e2e/v2/internal/workload_registry.go(1 hunks)test/e2e/v2/internal/workload_resolver.go(1 hunks)test/e2e/v2/tests/api_ux_validation_test.go(1 hunks)test/e2e/v2/tests/assets/hostedcluster-base.yaml(1 hunks)test/e2e/v2/tests/assets/karpenter-nodepool.yaml(1 hunks)test/e2e/v2/tests/assets/karpenter-workloads.yaml(1 hunks)test/e2e/v2/tests/assets/nodepool-base.yaml(1 hunks)test/e2e/v2/tests/control_plane_workloads_test.go(1 hunks)test/e2e/v2/tests/suite_test.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- test/e2e/v2/internal/workload_resolver.go
- test/e2e/v2/internal/env_vars.go
- test/e2e/v2/tests/assets/karpenter-workloads.yaml
- test/e2e/v2/tests/assets/karpenter-nodepool.yaml
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
test/e2e/util/crd.gotest/e2e/v2/tests/api_ux_validation_test.gotest/e2e/v2/tests/suite_test.gotest/e2e/util/version.gotest/e2e/v2/internal/test_context.gotest/e2e/v2/internal/workload_registry.gotest/e2e/v2/tests/control_plane_workloads_test.gotest/e2e/v2/tests/assets/nodepool-base.yamltest/e2e/v2/tests/assets/hostedcluster-base.yaml
Introduce a new Ginkgo v2 based end-to-end test suite with improved structure and organization. The new suite includes: - Suite setup with BeforeSuite/AfterSuite hooks for test initialization - Test organization under test/e2e/v2/tests/ directory - Internal utilities and helpers in test/e2e/v2/internal/ - CRD utility functions in test/e2e/util/ for cross-version compatibility This new test framework will enable more maintainable and scalable e2e testing with better parallel execution support and clearer test structure. Signed-off-by: Cesar Wong <cewong@redhat.com> Assisted-by: Claude 3.7 Sonnet (via Claude Code)
Contributor
Author
|
@coderabbitai Thank you for the thorough review! I've addressed all the identified issues: 1. Array traversal in CRD schema validation (test/e2e/util/crd.go:80) 2. Nil pointer guards in version utility (test/e2e/util/version.go:78) 3. CronJob workload resolution (test/e2e/v2/internal/workload_resolver.go:199) 4. Invalid DNS hostnames (test/e2e/v2/tests/assets/hostedcluster-base.yaml:49) 5. Loop variable capture (test/e2e/v2/tests/control_plane_workloads_test.go:184) 6. Nil guards for pod affinity checks (test/e2e/v2/tests/control_plane_workloads_test.go:698) 7. Duplicate test case (test/e2e/v2/tests/api_ux_validation_test.go:1317) All changes maintain backward compatibility while improving code robustness and test accuracy. AI-assisted response via Claude Code |
Contributor
|
Tip For best results, initiate chat on the files or code changes. @csrwng Thank you for the detailed update! I really appreciate the thoroughness of your fixes and the clear summary of how each issue was addressed. Your changes look solid:
The fixes enhance the robustness of the test suite while maintaining backward compatibility. Great work on getting this comprehensive v2 test suite ready! 🎉 |
csrwng
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@csrwng: This pull request explicitly references no jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
openshift-merge-bot
Bot
merged commit Nov 7, 2025
6644889
into
openshift:feat-e2e-v2
2 checks passed
6 tasks
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Introduce a new Ginkgo v2 based end-to-end test suite with improved
structure and organization. The new suite includes:
This new test framework will enable more maintainable and scalable e2e
testing with better parallel execution support and clearer test structure.
Signed-off-by: Cesar Wong cewong@redhat.com
Assisted-by: Claude 3.7 Sonnet (via Claude Code)